-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expanded code system preference lists #178
Conversation
@@ -8,7 +8,7 @@ | |||
class CodeDetectionBuilder(base_table_builder.BaseTableBuilder): | |||
display_text = "Selecting unique code systems..." | |||
|
|||
def _check_coding_against_db(code_source, schema, cursor): | |||
def _check_coding_against_db(self, code_source, schema, cursor): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to have tests for this study - #179
"urn:oid:1.2.840.114350.1.13.71.2.7.2", | ||
"urn:oid:1.2.840.114350.1.13.71.2.7.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these private/arbitrary systems need comments.
@@ -21,7 +21,7 @@ CREATE TABLE {{ target_table }} AS ( | |||
{%- endif %} | |||
{%- if filter_priority %} | |||
WHERE | |||
u.codeable_concept.system = '{{ system }}' | |||
u.codeable_concept.system LIKE '{{ system }}%' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh thats.. interesting. As a developer passing a system around, I would be very surprised to see it used as a prefix rather than a black-box string. http://example.org/animals/
being different from http://example.org/animals/lions
for example. What's the use case for this - private urn namespaces? Is there maybe a way to opt-into the wildcard nature? But even then, I'm leery that we get a promise about all possible meanings / systems in the namespace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a lot of noise in the numeric ones specifically, and without the context, I couldn't determine what they corresponded to and how comprehensive they were, so rather than putting in 15 codes i tried to cheat it - i can flip this back and be more explicit.
Edit - this will probably have to stick to LIKE to handle the site-specific guid, but i'll remove the hard coded wildcard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, per our conversation offline, i've updated this to the explicit IDs, and removed the installation-specific guids from the cerner IDs.
I created #180 to track the need to think about some optimizations as these lists get larger.
caf710c
to
53c7930
Compare
@@ -54,6 +54,12 @@ def denormalize_codes(self): | |||
"http://snomed.info/sct", | |||
"http://hl7.org/fhir/sid/icd-10-cm", | |||
"http://hl7.org/fhir/sid/icd-9-cm", | |||
"http://hl7.org/fhir/sid/icd-9-cm/diagnosis", | |||
# Cerner specific systems |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem entirely true - it's a spec standard way of indicating why data is gone. Epic doesn't use it though, it sounds like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh good catch
"http://hl7.org/fhir/sid/icd-9-cm/diagnosis", | ||
# Cerner specific systems | ||
"http://terminology.hl7.org/CodeSystem/data-absent-reason", | ||
# EPIC specific systems |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing we can't say anything else about them? But it'd be nice if we could distinguish what these two mean even a little bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, hard agree!
"http://hl7.org/fhir/sid/icd-10-cm", | ||
"http://hl7.org/fhir/sid/icd-9-cm", | ||
# Cerner specific systems | ||
"https://fhir.cerner.com/%/nomenclature", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔
This PR makes the following change:
Checklist
docs/
) needs to be updated